Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix api.derive.staking.eraExposure [WIP] #5813

Closed
wants to merge 7 commits into from
Closed

Conversation

TarikGul
Copy link
Member

@TarikGul TarikGul commented Mar 8, 2024

closes: #5771

The following PR ensures to update the api.derive.staking.eraExposure to use the right storage queries. For now I ensure that is is backwards compatible. Once the transition between storage entries is completely finished with polkadot, eraStakersClipped can be completely removed.

Once #5812 goes in this will have to be updated again as the types are then removed from the augmented polkadot and kusama files.

@TarikGul
Copy link
Member Author

TarikGul commented Mar 8, 2024

Few things need to be fixed in the derives that use eraExposure.

@TarikGul
Copy link
Member Author

TarikGul commented Mar 8, 2024

Still need to test this against the APPS and also figure out how to get a validators own, and total since those are now deprecated from PalletStakingExposure

@@ -48,8 +48,8 @@ function parseRewards (api: DeriveApi, stashId: AccountId, [erasPoints, erasPref
const valCut = valComm.mul(avail).div(BN_BILLION);
let staked: BN;

if (validatorId === stakerId) {
staked = exposure.own.unwrap();
if (validatorId === stakerId && (exposure as PalletStakingExposure).own) {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure if this is valid. Need to double check

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I need to add the storage call to ErasStakersOverview , and do this correctly so we don't need to come back to it.

@TarikGul TarikGul changed the title Fix api.derive.staking.eraExposure Fix api.derive.staking.eraExposure [WIP] Mar 9, 2024
@TarikGul
Copy link
Member Author

This PR brings up a good point for future derive work. What and how should compatibility be troubleshot and what is an acceptable change for upstream changes. Also how and when should they be released?

@TarikGul
Copy link
Member Author

TarikGul commented Mar 11, 2024

It honestly might be better to just create a whole new derive called eraExposurePaged which users can then migrate too when the time comes

@TarikGul
Copy link
Member Author

closing in favor of #5815

@TarikGul TarikGul closed this Mar 12, 2024
@polkadot-js-bot
Copy link

This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@polkadot-js polkadot-js locked as resolved and limited conversation to collaborators Mar 14, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Underlying storage for exposures has changed
2 participants